[client] fix ios network addresses mac filter#5906
Merged
Conversation
iOS does not expose hardware (MAC) addresses due to Apple's privacy restrictions (since iOS 14), causing networkAddresses() to return an empty list because all interfaces are filtered out by the HardwareAddr check. Move networkAddresses() to platform-specific files so iOS can skip this filter.
Link-local (fe80::) and multicast addresses are not useful for posture checks and add noise to the reported network addresses. Filter them out on iOS, consistent with how other parts of the codebase handle these.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved duplicated network-address logic from the generic info file and introduced platform-specific implementations: an iOS Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/system/info_ios.go`:
- Around line 73-74: The multicast filtering currently calls
ipNet.IP.IsLinkLocalMulticast(), which only excludes link-local multicast;
update the check to use ipNet.IP.IsMulticast() instead so all multicast ranges
are filtered out. Locate the loop in client/system/info_ios.go where
ipNet.IP.IsLoopback() / IsLinkLocalUnicast() are checked (the line containing
ipNet.IP.IsLinkLocalMulticast()) and replace that predicate with
ipNet.IP.IsMulticast(), keeping the surrounding continue logic unchanged.
In `@client/system/network_addr.go`:
- Around line 39-41: Replace the unsafe netip.MustParsePrefix call with
netip.ParsePrefix and handle parse errors to avoid panics: where netAddr is
constructed (the code using ipNet.String() to build a NetworkAddress) and the
similar usage in info_ios.go, call netip.ParsePrefix(ipNet.String()), check the
returned error, and if it fails log or ignore that interface entry and continue
(do not panic or add an invalid NetworkAddress); ensure the rest of the logic
that depends on the parsed prefix only runs when parsing succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fffb500e-85a3-4fe9-bcd3-70646c7d915f
📒 Files selected for processing (3)
client/system/info.goclient/system/info_ios.goclient/system/network_addr.go
💤 Files with no reviewable changes (1)
- client/system/info.go
lixmal
previously approved these changes
Apr 16, 2026
…afely - Replace IsLinkLocalMulticast() with IsMulticast() to filter all multicast ranges, not just link-local - Replace unsafe MustParsePrefix() with ParsePrefix() and handle errors gracefully by skipping invalid addresses instead of panicking
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/system/info_ios.go`:
- Around line 51-94: Extract the inner per-address loop of networkAddresses into
a helper (e.g., processInterfaceAddress(iface net.Interface, address net.Addr)
(*NetworkAddress, error)); the helper should perform the same checks currently
in the loop: assert address is *net.IPNet, skip loopback/link-local/multicast,
parse prefix via netip.ParsePrefix, build NetworkAddress with Mac set to
iface.HardwareAddr.String(), and return nil on any check/parse failure; keep
duplicate detection by calling isDuplicated(netAddresses, netAddr) from the
caller (or return a sentinel to indicate duplication) so networkAddresses can
remain responsible for appending unique entries and preserve the existing
MAC-check bypass, error handling, and overall behavior.
In `@client/system/network_addr.go`:
- Around line 10-56: The function networkAddresses exceeds Sonar complexity;
extract the inner per-interface/address logic into a small helper (e.g.,
processInterfaceAddresses or collectAddressesFromInterface) that takes an iface
(net.Interface) and the current []NetworkAddress and returns the updated slice
(or new addresses) and/or error; move the addr loop, type assertion to
*net.IPNet, loopback check, prefix parsing (netip.ParsePrefix), creation of
NetworkAddress (NetIP, Mac: iface.HardwareAddr.String()), the non-iOS MAC filter
(iface.HardwareAddr.String() == ""), and the isDuplicated check into that helper
so networkAddresses only iterates interfaces, calls the helper for each up
interface, and appends returned addresses, preserving existing behavior and
error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ba4b275-a023-4bc8-b983-7d65fdf42015
📒 Files selected for processing (2)
client/system/info_ios.goclient/system/network_addr.go
iOS returns a fixed 02:00:00:00:00:00 placeholder for HardwareAddr due to Apple's privacy restrictions. Setting Mac to "" matches Android's behavior and avoids sending a meaningless placeholder to the management server.
Lowers cognitive complexity of networkAddresses below the linter threshold by moving the per-address type-assertion, IP filtering, and prefix parsing into a small helper.
Lowers cognitive complexity of networkAddresses below the linter threshold by moving the per-address type-assertion, loopback filter, and prefix parsing into a small helper, mirroring the iOS implementation.
|
lixmal
approved these changes
Apr 20, 2026
MichaelUray
added a commit
to MichaelUray/netbird
that referenced
this pull request
Apr 24, 2026
…-addresses Resolved conflicts against upstream netbirdio#5906 (ios mac filter / network_addr.go extraction) and netbirdio#5888 (DebugBundle on Android client): - client/android/client.go: keep both OnUnderlyingNetworkChanged (ours) and DebugBundle (upstream), replaced non-ASCII arrow in comment with plain text. - client/system/info.go: drop duplicated networkAddresses/isDuplicated - they now live in client/system/network_addr.go after upstream extraction. - client/system/network_addr.go: adopt upstream's toNetworkAddress helper but keep ctx-aware signature + skipNoMacFilter so Android continues to use the external iFace discoverer. - client/system/info_ios.go: add exported NetworkAddresses(ctx) shim so the engine call compiles on ios; the iOS body stays context-free (iOS has no external discoverer).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
On iOS, networkAddresses() returned an empty list because Apple’s privacy restrictions (since iOS 14) prevent access to real hardware (MAC) addresses. As a result, all interfaces were filtered out by the HardwareAddr == "" check.
Split networkAddresses() into platform-specific implementations:
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit
Refactor
New Features